Skip to content

Security: remote-pack RCE, i18n XSS sink, credential and container hardening - #4

Open
phyersherman wants to merge 3 commits into
mainfrom
security/phase1-critical-fixes
Open

Security: remote-pack RCE, i18n XSS sink, credential and container hardening#4
phyersherman wants to merge 3 commits into
mainfrom
security/phase1-critical-fixes

Conversation

@phyersherman

Copy link
Copy Markdown
Collaborator

Phase 1 of the CIS v8.1 / NIST SSDF review. Reviewed and signed off by @Apomera; the DOMPurify item was resolved differently than originally proposed (see below).

What this fixes

Remote code execution in the language-pack loader (highest severity). App.jsx ran new Function('return ' + text)() on fetched pack bodies, so a compromised CDN — or anyone landing a file in lang/ on main — had arbitrary code execution in every client. All 63 shipped packs parse as pure JSON, so the eval was dead code for legitimate packs and only ever fired on malformed or malicious content. Removed.

XSS through the i18n layer. t() resolves the language pack before the static UI_STRINGS, so a pack can override any key, and several STEM-lab consumers concatenate t() output into innerHTML. Three untrusted writers reach the pack: a user-chosen file (validated only as "is an object"), a CDN fetch, and LLM translation output.

Fixed at the source rather than at the sinks. The per-sink approach would have patched 18 sinks in stem_lab_module.js — a file that is not the one deployed — while leaving 37 in stem_tool_solarsystem.js, which is what actually loads at runtime.

The sanitizer removes only executable elements. 749 shipped strings legitimately contain markup (<strong>, plus the a11y lab's deliberately-inaccessible <html>/<img> teaching samples), and it leaves all 1,654,508 strings across 63 packs byte-identical while neutralizing every payload tested.

Credentials off Math.random() — anonymous account password, live-session code, LAN join PIN. Rejection sampling keeps the output uniform; a chi-square test over 2M draws confirms no modulo bias.

Gemini key out of URL query strings (24 sites). URLs reach browser history, proxy logs, and Referer headers. utils_pure already stripped ?key=… from error strings, which was the tell these URLs were being captured. The key is added only to generativelanguage.googleapis.com requests — the local Flux server and localhost Edge TTS endpoints keep plain headers.

Also: loopback-only CORS + bind on the standalone TTS servers, 0600 on the School Box env file, PowerShell paths passed as arguments instead of spliced into a command string, and CPU/memory ceilings on all 19 compose services (generous and env-overridable — a cap tight enough to OOM-kill inference would be worse than the exhaustion it prevents).

Not fixed

The SearXNG secret_key stays committed, with the constraint documented. It cannot move to an env var: SearXNG resolves settings only from YAML (settings_loader.py honors SEARXNG_SETTINGS_PATH and nothing else), and the image's sed-substitution needs a writable settings.yml while ours is mounted read-only. It signs that instance's own session cookies and is not a credential to anything external.

Verification

Baseline measured at origin/main (1fa420161): 442 failing test files, 26 failing tests. This branch is identical — zero new failures — plus 48 new passing tests across two new suites.

The new tests pin both halves of each fix. For the key migration that matters: removing the key from the URL without adding the header would leave every call unauthenticated and looks identical in a diff.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
alloflow-cdn 0370a81 Jul 31 2026, 11:08 PM

@Apomera

Apomera commented Jul 30, 2026 via email

Copy link
Copy Markdown
Owner

@phyersherman

phyersherman commented Jul 30, 2026 via email

Copy link
Copy Markdown
Collaborator Author

Hubby and others added 3 commits July 31, 2026 19:01
…al paths

Language packs are untrusted input: they arrive from a user-chosen file, a CDN
fetch, or LLM translation output, and t() resolves them BEFORE the static
UI_STRINGS, so a pack can override any key.

- Drop the `new Function('return ' + text)` fallback in the pack loader. A
  compromised pack host had arbitrary code execution in every client. All 63
  shipped packs parse as pure JSON, so the eval was dead code for real packs.
- Sanitize every pack ingest (import, cached, remote, AI partial/final). Only
  executable elements are removed; the a11y lab's deliberately-bad HTML samples
  and inline <strong> markup are preserved byte-identical across all 63 packs.
  Attribute scrubbing is confined to tag markup so prose like "10 ones = 1 ten"
  is untouched, and "/" is treated as an attribute separator (<svg/onload=...>).
- Replace Math.random() for the anonymous account password, the live-session
  code, and the LAN join PIN with CSPRNG draws. Rejection sampling keeps the
  output uniform; chi-square over 2M draws confirms no modulo bias.
- Restrict CORS on the standalone TTS servers to loopback origins and bind
  edge-tts to 127.0.0.1, matching piper.
- Write the School Box env file 0600 and chmod pre-existing ones.
- Pass Compress-Archive paths as arguments rather than splicing them into a
  PowerShell command string.

Full suite matches the pre-existing baseline exactly (539 failing files, 133
failing tests, all present on clean main) plus 28 new passing tests.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ring

Gemini accepts the key either way. The query-string form leaks: URLs reach
browser history, proxy and server access logs, and Referer headers — places the
request body never goes. utils_pure already stripped "?key=…" from error
strings, which was the tell that these URLs were being captured.

Moves all 24 call sites to the x-goog-api-key header across src/aiProvider.js,
ai_backend_module.js, tts_module.js and utils_pure_module.js, plus their
*_source.jsx originals and desktop/web-app/public mirrors. aiProvider and
ai_backend route through a shared _geminiHeaders() helper.

The key is added ONLY to generativelanguage.googleapis.com requests. The local
Flux image server and the localhost Edge TTS endpoints in ai_backend keep
plain headers — sending a user's Google key to another process is the failure
this change is meant to avoid, not introduce. Retry paths carry the header too;
the TTS retry was a separate call site from its parent request.

Removing the key from the URL without adding the header would leave every call
unauthenticated and look identical in a diff, so the new test pins both halves,
across every duplicated copy, and asserts the non-Google endpoints stay clean.

Updates the request-shape fixtures to the new contract (they pinned the old
?key= URL) and re-stamps the tts_module.js content hash in the host loader.

Full suite matches the pre-existing baseline exactly: 539 failing files, 133
failing tests, all present on clean main. No new failures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…aint

Resource limits (CIS 16.7)
--------------------------
No compose service declared deploy.resources.limits, so any one container could
consume the host's CPU and memory — an ollama or flux run that grows without
bound takes the whole School Box down with it, including PocketBase.

Adds cpu/memory ceilings to all 19 services across the three compose files. The
existing GPU device reservations on ollama and flux are preserved alongside the
new limits.

The values are deliberately generous and env-overridable (OLLAMA_MEMORY etc.,
documented in the .env.example files). These are safety ceilings, not workload
sizing: a limit tight enough to OOM-kill legitimate inference would be a worse
outcome than the exhaustion it prevents. Ollama and flux hold model weights in
RAM, so their defaults are set high and are the ones to raise for large models.

Verified by parsing all three files: every service carries limits, GPU
reservations survive, and the top-level volumes key is intact.

SearXNG secret
--------------
Left in place, with the constraint documented. Moving it to an env var is not
possible for this image: SearXNG resolves settings only from YAML
(settings_loader.py honors SEARXNG_SETTINGS_PATH and nothing else — there is no
env interpolation), and the upstream sed-substitution trick needs a writable
settings.yml while ours is mounted read-only. The comment now records what the
key actually signs (this instance's own session cookies — not a credential to
any account or external service) and how to supply a per-install value by
mounting a replacement.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@phyersherman
phyersherman force-pushed the security/phase1-critical-fixes branch from 9c8d60b to 0370a81 Compare July 31, 2026 23:07
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 31, 2026

Copy link
Copy Markdown

Deploying alloflow-cdn with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0370a81
Status: ✅  Deploy successful!
Preview URL: https://f2d0538d.alloflow-cdn.pages.dev
Branch Preview URL: https://security-phase1-critical-fix.alloflow-cdn.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants